-
Notifications
You must be signed in to change notification settings - Fork 1.3k
UI: Improve error handling for template upload notifications #12412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Improve error handling for template upload notifications #12412
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12412 +/- ##
=========================================
Coverage 16.24% 16.24%
- Complexity 13378 13381 +3
=========================================
Files 5657 5657
Lines 498971 498973 +2
Branches 60561 60561
=========================================
+ Hits 81033 81035 +2
- Misses 408904 408905 +1
+ Partials 9034 9033 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vishesh92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm. didn't test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves error handling for template upload failures by properly extracting and displaying error messages from the SSVM (Secondary Storage VM). Instead of showing a generic "Request failed with status code 400" message, the UI now displays the actual error message from the backend.
Changes:
- Replaced custom error notification in template upload with
$notifyError()utility - Refactored error message extraction logic to prioritize response headers and data over generic error messages
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/src/views/image/RegisterOrUploadTemplate.vue | Simplified error handling to use $notifyError() utility |
| ui/src/utils/plugins.js | Improved error message extraction logic to check response data before falling back to generic error message |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| desc = error.response.headers['x-description'] | ||
| } | ||
| if (desc === '' && error.response.data) { | ||
| } else if (error.response.data) { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition check else if (error.response.data) is too broad and will evaluate to true for empty objects or arrays. This could lead to unnecessary processing of the response data block when there's no actual data. Consider using a more specific check like else if (error.response.data && Object.keys(error.response.data).length > 0) or checking for truthy values that actually contain data.
| } else if (error.response.data) { | |
| } else if (error.response.data && (typeof error.response.data !== 'object' || Object.keys(error.response.data).length > 0)) { |
| } else if (typeof error.response.data === 'string') { | ||
| desc = error.response.data |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String responses should be trimmed before assignment to avoid displaying empty or whitespace-only error messages. Consider using desc = error.response.data.trim() and only assigning if the trimmed result is non-empty.
|
@RosiKyu a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
@blueorangutan package |
|
@RosiKyu a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16485 |
RosiKyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
TC1: Template Upload - Format Mismatch Error Message
Objective: Verify UI displays detailed error message when uploaded file format doesn't match selected format
Expected Result: UI shows descriptive error message with detected file type instead of generic "Request failed with status code 500"
Actual Result: Success - UI displays "File type mismatch between the sent file and the actual content. Received: PNG image data, 2402 x 1723, 8-bit/color RGBA, non-interlaced"
Test Evidence:
TC2: Valid Template Upload - Regression Test
Objective: Verify valid QCOW2 template uploads successfully after the fix (no regression)
Expected Result: Template uploads and becomes Ready
Actual Result: Success - Template uploaded successfully and shows Ready state
Test Evidence:
Description
This PR fixes template upload error messages not being displayed in the UI. Previously, when template uploads failed (e.g., file format mismatch), the UI would show a generic "Request failed with status code 400" message instead of the actual error message from the SSVM (Secondary Storage VM).
Fixes: #11513
Types of changes
Bug Severity
Screenshots (if appropriate):
Broken:
Fixed:
How Has This Been Tested?